Zwave Lock: Support SLGA Migration by hcarter-775 · Pull Request #2944 · SmartThingsCommunity/SmartThingsEdgeDrivers

hcarter-775 · 2026-05-04T14:16:51Z

Description of Change

Follow-up to #2937, where I've responded to some comments that myself and others have left in order to clarify certain functionality.

Note on structure: The legacy-handlers subdriver is nearly untouched from before- the only real difference is that the key-we logic was lifted out from there since it is so close in from to the "new", now default key-we overriden behavior.

The other primary thing that was removed from the now-legacy-handlers subdriver is the old added handler, for the following 2 reasons. 1. Generally, devices should not be added to this driver any longer, and 2. We now use the added main driver implementation to initially set the migrated keyword to true.

I find the fact that the entire migrated state depends on a keyword set only once in added to be a little precarious, but nothing stands out to me initially at this not working.

Summary of Completed Tests

github-actions · 2026-05-04T14:17:21Z

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions · 2026-05-04T14:18:12Z

Invitation URL:
https://bestow-regional.api.smartthings.com/invite/q0jmxkaLPYl1

github-actions · 2026-05-04T14:19:19Z

Test Results

72 files 513 suites 0s ⏱️
2 846 tests 2 846 ✅ 0 💤 0 ❌
4 724 runs 4 724 ✅ 0 💤 0 ❌

Results for commit f24f62f.

♻️ This comment has been updated with latest results.

github-actions · 2026-05-04T14:19:21Z

File	Coverage
All files	`89%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/zwave-alarm-v1-lock/init.lua	`80%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/apiv6_bugfix/can_handle.lua	`66%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/init.lua	`88%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/zwave_lock_utils.lua	`85%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/lazy_load_subdriver.lua	`57%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/samsung-lock/init.lua	`94%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/schlage-lock/init.lua	`95%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/legacy-handlers/samsung-lock/init.lua	`94%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/legacy-handlers/schlage-lock/init.lua	`95%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/legacy-handlers/zwave-alarm-v1-lock/init.lua	`89%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zwave-lock/src/keywe-lock/init.lua	`80%`	❌

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against f24f62f

nickolas-deboom · 2026-05-06T17:30:13Z

For some reason my comment is failing to save when i try to add an in line comment, but I was wondering if in addition to the lockCodes.migrated attribute should we also set a persisted flag to indicated the migrated state? Just since otherwise it would fallback to the legacy handling if the state cache is cleared

cjswedes · 2026-05-06T19:00:19Z

For some reason my comment is failing to save when i try to add an in line comment, but I was wondering if in addition to the lockCodes.migrated attribute should we also set a persisted flag to indicated the migrated state? Just since otherwise it would fallback to the legacy handling if the state cache is cleared

I agree with this: we should use the datastore. The state should always be available now that we are using the hubs state store to back the driver state_cache, but we have not always had this guarantee, and the pattern of expecting cached state to be present is not good IMO. Should we disable that for this driver, or make a change to how the hub handles state, it could have unintended consequences here.

hcarter-775 · 2026-05-06T21:47:01Z

@cjswedes @nickolas-deboom
Agreed, makes sense to me.

If we're wondering about the state cache, I wonder how secure lockUsers and lockCredentials are as storage places for all this data. Should we be storing that information via the datastore as well?

nickolas-deboom · 2026-05-08T19:09:29Z

    version: 1
  - id: lockCodes
    version: 1
+  - id: lockCredentials


What if an existing device on one of these profiles that would not be migrated doesn't support these capabilities? I'm just wondering if new profiles should be created instead

nickolas-deboom · 2026-05-08T19:12:24Z

  zwave_handlers = {
    [cc.TIME] = {
-      [Time.GET] = time_get_handler -- used by DanaLock
+      [0x01] = time_get_handler -- used by DanaLock


Just curious but why did Time.GET change to a hardcoded value?

cjswedes · 2026-05-08T20:24:48Z

-  end
-end

-local function populate_state_from_data(device)


This function was removed from the legacy init handler. I think it needs to be included in the legacy path so we keep functionality the same for devices that are not yet migrated.

cjswedes · 2026-05-08T20:34:45Z

+
+  if status == lock_utils.STATUS_SUCCESS then
+    device:set_field(lock_utils.ACTIVE_CREDENTIAL,
+      { userIndex = user_index, userType = user_type, credentialType = credential_type, credentialIndex = credential_index })


We should include the credentialName data here as well so it is not lost. A few other places would need to be updated as well (update credential, and add_credential for sure)

cjswedes · 2026-05-08T20:44:54Z

@@ -151,6 +293,8 @@ local driver_template = {
  supported_capabilities = {
    capabilities.lock,


The default lock handlers use codeId and codeName and do not include the userIndex, userName, or userType fields. I think for clarity, we should override these handlers in the base driver, and stop using the default handlers here; doing so will have implications on the legacy handlers too though, we would have to make sure that subdriver still includes the legacy event fields.

cjswedes · 2026-05-08T20:48:18Z

+  for _, user in pairs(current_users) do
+    used_index[user.userIndex] = true
+  end
+  if current_users ~= {} then


This will always be true.

Suggested change

if current_users ~= {} then

if utils.table_size(current_users) == 0 then

cjswedes · 2026-05-08T20:49:26Z

+  for _, credential in pairs(current_credentials) do
+    used_index[credential.credentialIndex] = true
+  end
+  if current_credentials ~= {} then


Suggested change

if current_credentials ~= {} then

if utils.table_size(current_credentials) == 0 then

ctowns · 2026-05-08T21:04:59Z

+  BUSY = "busy",
+  COMMAND_IN_PROGRESS = "commandInProgress",
+  CREDENTIAL_TYPE = "pin",
+  CHECKING_CODE = "checkingCode",


Could you add a brief comment explaining what checkingCode is supposed to be used for? It can be a little confusing to understand without parsing through the code.

ctowns · 2026-05-08T21:08:10Z

+
+local DEFAULT_SUPPORTED_PIN_SLOTS = 8
+
+-- check if we are currently busy performing a task.


Could we add more documentation on why the busy state is used/neccessary here? This is a source of complexity in both of these lock drivers and I think better documentation would help with reviews and future maintainability.

cbaumler and others added 3 commits May 3, 2026 14:09

CHAD-16364: Update Z-Wave lock capabilities

fc30950

Remove stale test accidentally included during rebase

360c9d1

Restructure zwave-lock

f746993

hcarter-775 requested review from aleclorimer, cbaumler, cjswedes, ctowns and nand-nor May 4, 2026 14:16

hcarter-775 requested a review from tpmanley May 4, 2026 14:30

only update on added if state is typed

7b30cc6

hcarter-775 requested a review from nickolas-deboom May 5, 2026 15:27

hcarter-775 added 3 commits May 6, 2026 17:53

update test file names, add slga migration field for SOT

2a3bf84

remove persistence on fields that are set on init

5016ced

fixup naming, fix failing command.name checks

f24f62f

hcarter-775 force-pushed the re-structure/feature/chad-16364-zwave branch from f5ab265 to f24f62f Compare May 8, 2026 15:47

nickolas-deboom reviewed May 8, 2026

View reviewed changes

cjswedes reviewed May 8, 2026

View reviewed changes

ctowns reviewed May 8, 2026

View reviewed changes

		@@ -151,6 +293,8 @@ local driver_template = {
		supported_capabilities = {
		capabilities.lock,

	if current_users ~= {} then
	if utils.table_size(current_users) == 0 then

	if current_credentials ~= {} then
	if utils.table_size(current_credentials) == 0 then


		local DEFAULT_SUPPORTED_PIN_SLOTS = 8

		-- check if we are currently busy performing a task.

Conversation

hcarter-775 commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Summary of Completed Tests

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickolas-deboom commented May 6, 2026

Uh oh!

cjswedes commented May 6, 2026

Uh oh!

hcarter-775 commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hcarter-775 commented May 4, 2026 •

edited

Loading

github-actions Bot commented May 4, 2026 •

edited

Loading

github-actions Bot commented May 4, 2026 •

edited

Loading